Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Use the prepass normal texture in main pass when possible #8231

Merged
merged 3 commits into from
Mar 29, 2023
Merged

Use the prepass normal texture in main pass when possible #8231

merged 3 commits into from
Mar 29, 2023

Conversation

IceSentry
Copy link
Contributor

@IceSentry IceSentry commented Mar 27, 2023 •
edited
Loading

Objective

  • We support enabling a normal prepass, but the main pass never actually uses it and recomputes the normals in the main pass. This isn't ideal since it's doing redundant work.

Solution

  • Use the normal texture from the prepass in the main pass

Notes

I used NORMAL_PREPASS_ENABLED as a shader_def because NORMAL_PREPASS is currently used to signify that it is running in the prepass while this shader_def need to indicate the prepass is done and the normal prepass was ran before. I'm not sure if there's a better way to name this.

@IceSentry IceSentry added A-Rendering Drawing game state to the screen C-Feature A new feature, making something new possible labels Mar 27, 2023
superdump
superdump approved these changes Mar 27, 2023
View reviewed changes
Copy link
Contributor

@superdump superdump left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suppose maybe LOAD_PREPASS_NORMALS could be clearer?

@IceSentry
Copy link
Contributor Author

Oh yeah, good call, I'll rename it to that

JMS55
JMS55 approved these changes Mar 28, 2023
View reviewed changes
Copy link
Contributor

@JMS55 JMS55 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yay GBuffer :)

@james7132 james7132 added the S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it label Mar 28, 2023
@james7132 james7132 added this pull request to the merge queue Mar 29, 2023
Merged via the queue into bevyengine:main with commit 0859f67 Mar 29, 2023
@IceSentry IceSentry deleted the prepass-normal-main-pass branch March 31, 2023 17:25
@JMS55 JMS55 mentioned this pull request Jun 3, 2023
Closed
This was referenced Jun 28, 2023
Closed
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@JMS55 JMS55 JMS55 approved these changes

@james7132 james7132 james7132 approved these changes

@superdump superdump superdump approved these changes

Assignees
No one assigned
Labels
A-Rendering Drawing game state to the screen C-Feature A new feature, making something new possible S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@IceSentry @superdump @james7132 @JMS55